feat(gitops-backend): Configure TLS MinVersion and Ciphers#1227
Conversation
…nd component Signed-off-by: akhil nittala <nakhil@redhat.com>
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe controller now receives a TLS profile and passes its minimum TLS version and cipher suites to the GitOps backend Deployment through environment variables. ChangesTLS backend configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controllers/gitopsservice_controller.go (1)
803-814: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse a switch statement instead of a map.
To avoid allocating a map on every function call, consider using a
switchstatement. It is more idiomatic and performs better.♻️ Proposed refactor
func TLSVersionToBackend(tlsVersion string) string { - versionMap := map[string]string{ - "VersionTLS12": "1.2", - "VersionTLS13": "1.3", - "VersionTLS11": "1.1", - "VersionTLS10": "1", - } - if v, ok := versionMap[tlsVersion]; ok { - return v - } - return "" // default fallback + switch tlsVersion { + case "VersionTLS12": + return "1.2" + case "VersionTLS13": + return "1.3" + case "VersionTLS11": + return "1.1" + case "VersionTLS10": + return "1" + default: + return "" // default fallback + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/gitopsservice_controller.go` around lines 803 - 814, Update TLSVersionToBackend to replace the per-call versionMap allocation with a switch on tlsVersion, returning the existing TLS version strings for VersionTLS10 through VersionTLS13 and preserving the empty-string fallback for unknown values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@controllers/gitopsservice_controller.go`:
- Around line 803-814: Update TLSVersionToBackend to replace the per-call
versionMap allocation with a switch on tlsVersion, returning the existing TLS
version strings for VersionTLS10 through VersionTLS13 and preserving the
empty-string fallback for unknown values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 21920b77-0290-4a4e-877d-7c69f5b22609
📒 Files selected for processing (2)
cmd/main.gocontrollers/gitopsservice_controller.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
…nd component Signed-off-by: akhil nittala <nakhil@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controllers/gitopsservice_controller_test.go`:
- Around line 113-126: In controllers/gitopsservice_controller_test.go sites
113-126 and 133-146, update both environment-variable loops to remove the early
break statements so TLS_MIN_VERSION and TLS_CIPHER_SUITES are always collected.
Separate the assertions in each test so gotTLSMinVersion is independently
checked against "1.3" and gotTLSCiphers is independently checked against
"dummy1" at 113-126 and "dummy1:dummy2" at 133-146.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 68ef1871-9cc1-442b-9fea-2370e80ce23a
📒 Files selected for processing (1)
controllers/gitopsservice_controller_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
|
/retest-required |
1 similar comment
|
/retest-required |
|
/retest-required |
|
/retest |
Signed-off-by: akhil nittala <nakhil@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/main.go`:
- Line 315: Update the GitOps service reconciler initialization in main to
assign the fetched profile from the existing profile variable to
CentralTLSProfile instead of an empty TLSProfileSpec. Preserve the fetched
cluster TLS profile so newBackendDeployment can emit the corresponding TLS
environment variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b96129c-53b2-4d7f-802f-aad391bfcc38
📒 Files selected for processing (3)
cmd/main.gocontrollers/gitopsservice_controller.gocontrollers/gitopsservice_controller_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- controllers/gitopsservice_controller_test.go
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: svghadi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
276d5bf
into
redhat-developer:master
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
To make gitops backend component complaint with OCP 5.0 goals, making tls minversion and ciphers configurable.
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
https://redhat.atlassian.net/browse/GITOPS-10474
Test acceptance criteria:
How to test changes / Special notes to the reviewer: